fix: preserve HTTP status for non-JSON API errors#238
Open
mayankbohradev wants to merge 1 commit into
Open
Conversation
When the response body is not usable JSON (CDN HTML, empty 5xx, proxies), use the real HTTP status for 4xx/5xx instead of always raising a fake 500. Mirror sync and async request paths.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #238 +/- ##
===========================================
+ Coverage 82.66% 95.96% +13.29%
===========================================
Files 4 61 +57
Lines 75 3221 +3146
===========================================
+ Hits 62 3091 +3029
- Misses 13 130 +117 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When the API (or a proxy/CDN in front of it) returns a non-JSON body, the SDK no longer always raises a fake
500 InternalServerError.It now uses:
status >= 400(e.g.429,401,503)500only when the status looks successful but the body still is not usable JSONApplies to both
request.pyandasync_request.py. Error type isapplication_error(aligned with the Node SDK’s non-JSON error path).Why
HTTP clients already return
(content, status_code, headers), but the request layer discardedstatus_codeand hard-coded500whenever:Content-Typewas notapplication/json, orjson.loadsfailedSo a real rate-limit / unauthorized / gateway HTML page became an unhelpful internal server error. Callers could not branch on
e.codeor useretry-afterfrom headers correctly.JSON error payloads that include
statusCode/name/messageare unchanged (perform()still handles those).Testing
tests/request_test.py:429+text/html→code == 429, headers preserved503+ non-JSON →code == 503200+ HTML → stillcode == 500502→code == 502401+ HTMLemails_testcase that patchedmake_requestand never hit this pathpytest tests/request_test.py tests/emails_test.py(482 suite tests also green viatox -e py)tox -e mypygreentox -e lintfails on Python 3.13 (flake8<5+importlib_metadata); CI still runs lint on 3.8–3.11Out of scope
Does not change behavior when a JSON body parses successfully but omits
statusCodewhile the HTTP status is 4xx/5xx. That is a separate issue.Summary by cubic
Preserves real HTTP status codes for non‑JSON API responses in both sync and async requests. Fixes false 500s so clients can react to 4xx/5xx correctly and use headers like Retry-After.
resend/request.pyandresend/async_request.py; added sync/async tests; removed a stale emails test.Written for commit 8451c2d. Summary will update on new commits.